Skip to content

Comments

[SPARK-9728][SQL]Support CalendarIntervalType in HiveQL#8034

Closed
yjshen wants to merge 2 commits intoapache:masterfrom
yjshen:interval_hiveql
Closed

[SPARK-9728][SQL]Support CalendarIntervalType in HiveQL#8034
yjshen wants to merge 2 commits intoapache:masterfrom
yjshen:interval_hiveql

Conversation

@yjshen
Copy link
Member

@yjshen yjshen commented Aug 7, 2015

This PR enables converting interval term in HiveQL to CalendarInterval Literal.

JIRA: https://issues.apache.org/jira/browse/SPARK-9728

rxin
rxin reviewed Aug 7, 2015
View reviewed changes
unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java Outdated
Copy link
Contributor

@rxin rxin Aug 7, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you declare the possible exceptions here?

@rxin
Copy link
Contributor

rxin commented Aug 7, 2015

We should also add unit tests for CalendarInterval itself.

@marmbrus
Copy link
Contributor

marmbrus commented Aug 7, 2015

It would also be good to actually fill out the description instead of just linking the JIRA, since this becomes the commit message.

@SparkQA
Copy link

SparkQA commented Aug 7, 2015

Test build #40175 has finished for PR 8034 at commit fce7795.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@yjshen
Copy link
Member Author

yjshen commented Aug 8, 2015

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Aug 8, 2015

Test build #40215 timed out for PR 8034 at commit 7fe9a5e after a configured wait of 175m.

@SparkQA
Copy link

SparkQA commented Aug 8, 2015

Test build #1413 has finished for PR 8034 at commit 7fe9a5e.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@yjshen
Copy link
Member Author

yjshen commented Aug 8, 2015

Failure due to org.apache.spark.network.sasl.SaslIntegrationSuite.testNoSaslClient

@SparkQA
Copy link

SparkQA commented Aug 8, 2015

Test build #1412 has finished for PR 8034 at commit 7fe9a5e.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@rxin
Copy link
Contributor

rxin commented Aug 8, 2015

Going to merge this. Thanks!

asfgit pushed a commit that referenced this pull request Aug 8, 2015
@yjshen @rxin
[SPARK-9728][SQL]Support CalendarIntervalType in HiveQL
3c438c7 
This PR enables converting interval term in HiveQL to CalendarInterval Literal.

JIRA: https://issues.apache.org/jira/browse/SPARK-9728

Author: Yijie Shen <henry.yijieshen@gmail.com>

Closes #8034 from yjshen/interval_hiveql and squashes the following commits:

7fe9a5e [Yijie Shen] declare throw exception and add unit test
fce7795 [Yijie Shen] convert hiveql interval term into CalendarInterval literal

(cherry picked from commit 23695f1)
Signed-off-by: Reynold Xin <rxin@databricks.com>
@asfgit asfgit closed this in 23695f1 Aug 8, 2015
CodingCat pushed a commit to CodingCat/spark that referenced this pull request Aug 17, 2015
@yjshen @CodingCat
[SPARK-9728][SQL]Support CalendarIntervalType in HiveQL
ee2d3ac 
This PR enables converting interval term in HiveQL to CalendarInterval Literal.

JIRA: https://issues.apache.org/jira/browse/SPARK-9728

Author: Yijie Shen <henry.yijieshen@gmail.com>

Closes apache#8034 from yjshen/interval_hiveql and squashes the following commits:

7fe9a5e [Yijie Shen] declare throw exception and add unit test
fce7795 [Yijie Shen] convert hiveql interval term into CalendarInterval literal
cloud-fan
cloud-fan reviewed Oct 27, 2015
View reviewed changes
unsafe/src/main/java/org/apache/spark/unsafe/types/CalendarInterval.java
Copy link
Contributor

@cloud-fan cloud-fan Oct 27, 2015

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not make unit Enum instead of String?

Copy link
Member

@MaxGekk MaxGekk Nov 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good question. Strings spreads everywhere now. @cloud-fan How about introducing Scala enum (since the code has been moved to IntervalUtils) for interval units for now.

Copy link
Contributor

@cloud-fan cloud-fan Nov 10, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SGTM

@MaxGekk MaxGekk mentioned this pull request Oct 10, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MaxGekk MaxGekk MaxGekk left review comments

@cloud-fan cloud-fan cloud-fan left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@yjshen @rxin @marmbrus @SparkQA @MaxGekk @cloud-fan